Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

fix(alert): use interpolation for type attribute #1460

Closed
wants to merge 1 commit into from

Conversation

bekos
Copy link
Contributor

@bekos bekos commented Dec 24, 2013

No description provided.

@bealtine
Copy link

bealtine commented Jan 4, 2014

I've forked and used type: @ as that seems more relevant to my usage requirements..(with added sanity checking), I tend to see the alert types as semi-constants (with a possibility to update)

@pkozlowski-opensource
Copy link
Member

@bekos @chrisirhc OK, I got you guys. And I totally agree with @chrisirhc that default should go to the template.
I'm going to merge it as soon as @chrisirhc lands 1.2 support.

@bekos
Copy link
Contributor Author

bekos commented Jan 8, 2014

@pkozlowski-opensource I will fix for 1.2 before. About the type I believe is more a matter of a consistency. Since we provide an attribute from our API, I think we should provide a default value through a config object, as we do in all the other directives. I will remove for now an open a new issue to discuss.

@pkozlowski-opensource
Copy link
Member

@bekos ok, let's do this!

@bekos
Copy link
Contributor Author

bekos commented Jan 18, 2014

@pkozlowski-opensource ping :-)

BREAKING CHANGE: Use interpolation for type attribute.

  Before:

  <alert type="'info'" ...></alert >
  or
  <alert type="alert.type" ...></alert >

  After:

  <alert type="info" ...></alert >
  or
  <alert type="{{alert.type}}" ...></alert >
@bekos bekos closed this in f0a129a Jan 21, 2014
jdbeutel added a commit to jdbeutel/ics699-bendy that referenced this pull request May 23, 2014
The update includes this breaking change:  angular-ui/bootstrap#1460  so I had to change the alert type attribute to interpolated.  I made the same change to bendy-alert-icon for consistency.

I did not include the update of the template-less versions, because I'm not sure if I am going to use them.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants